-
Notifications
You must be signed in to change notification settings - Fork 1
Driver: Improve section about Java #402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughRemoves the legacy consolidated Java JDBC page; adds a reorganized Java connect section split into multiple driver/ORM/testing pages; updates connect navigation to Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Areas to pay extra attention:
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧠 Learnings (4)📚 Learning: 2025-10-25T22:54:24.865ZApplied to files:
📚 Learning: 2025-10-06T16:40:16.322ZApplied to files:
📚 Learning: 2025-10-19T19:21:49.864ZApplied to files:
📚 Learning: 2025-10-08T01:34:18.867ZApplied to files:
🪛 markdownlint-cli2 (0.18.1)docs/connect/java/cratedb-jdbc.md85-85: Bare URL used (MD034, no-bare-urls) 92-92: Bare URL used (MD034, no-bare-urls) docs/connect/java/postgresql-jdbc.md85-85: Bare URL used (MD034, no-bare-urls) 92-92: Bare URL used (MD034, no-bare-urls) 🔇 Additional comments (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
494a5ae to
5e4c848
Compare
seut
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx, left some suggestions.
docs/connect/java/cratedb-jdbc.md
Outdated
| Invoke-WebRequest https://repo1.maven.org/maven2/io/crate/crate-jdbc-standalone/2.7.0/crate-jdbc-standalone-2.7.0.jar -OutFile crate-jdbc-standalone-2.7.0.jar | ||
| ``` | ||
| ::: | ||
| Invoke program. Needs Java >= 25 ([JEP 330]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crate-jdbc requires >= 11 (https://github.com/crate/crate-jdbc/blob/master/build.gradle#L19). Also the given command does not required JEP 330 afaik.
| Invoke program. Needs Java >= 25 ([JEP 330]). | |
| Invoke program. Needs Java >= 11. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JEP 330
Hi. We see that JEP 330 landed with Java 11 already, so this comment is certainly misguided; thanks for spotting it.
Apparently, the fourth mode for the Java launcher to launch a class declared in a source file has been added a long time ago already.
java HelloWorld.java
We may have been confused about it, because we don't launch Java programs so often, so we didn't know about this option before, expecting that launching a program requires an incantation of javac beforehand.
JEP 445
However, the example above uses this feature already:
Java 21 introduces unnamed classes to make the
classdeclaration implicit. Alsopublic static void main(String args[])is no longer required, to reduce boilerplate for single-class programs.
So, this effectively needs JEP 445 which landed with Java 21. Comparing those snippets, it saves a few bytes, class noise, a whole indentation level, and any "naming things" obstacles regarding Java specifics in file name vs. class name discrepancies.
public class HelloWorld {
public static void main(String[] args) {
System.out.println("Hello, World!");
}
}void main() {
System.out.println("Hello, World!");
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts / Evaluation
So, is it worth to sacrifice backward compatibility for the canonical poster example?
My thoughts were we indeed could do it, because the other spot where canonical code examples are presented, in this case cratedb-examples:by-language/java-jdbc, displays a full Java program BasicPostgresCrateDB.java anyway.
In this spirit, we wanted to reduce boilerplate to the max here, yet retaining the idea of presenting an executable example. It's a narrow path which sometimes needs modern language features, or utility programs like uv for elegantly launching Python code snippets, where the user/reader mostly needed to whip up a whole programming environment (Python virtualenv) before even getting started.
Following this idea and the modernization efforts of upstream software authors who work on such ergonomic enhancements, we have been aiming to present "modern" variants of how to minimally invoke arbitrary code snippets on our documentation as well.
Evaluation / Next steps
We can collectively decide if it's okay to present the modern variants while sacrificing backwards compatibility with previous programming language versions, but sure we should present information which accompanies older versions as well.
Let me come up with another iteration on this document to accomodate a better balance for Java < 21, so you can evaluate and decide if you like it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crate-jdbc requires >= 11
We will certainly add this remark about the effective runtime dependency separately and more prominently, thank you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed with 0fff874 and dcce170, then ba1ed79, thank you.
At the top, the page now says:
CrateDB JDBC needs Java >= 11.
Only at the bottom when it's about invoking the code snippet at hand, which is primarily thought to be a descriptive example 1, it says:
Invoke program. Needs Java >= 21 (JEP 445), alternatively see Full example.
Footnotes
-
[...] with an optional benefit that it's actually also executable. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth to sacrifice backward compatibility for the canonical poster example?
My thoughts were we indeed could do it, because the other spot where canonical code examples are presented, in this case cratedb-examples:by-language/java-jdbc, displays a full Java program BasicPostgresCrateDB.java anyway, [and it is referenced right at the bottom of the page].
The adjustment 5d5de24 further makes this less of an issue, by bringing both spots much closer together now.
Let me also correct my previous statements about JEP 445: The minimal code example needs JEP 512, which is in effect with Java 25.
docs/connect/java/index.md
Outdated
| - {ref}`postgresql-jdbc` — `jdbc:postgresql://` | ||
| - {ref}`cratedb-jdbc` — `jdbc:crate://` | ||
|
|
||
| Prefer the PostgreSQL JDBC driver first—it’s often already on your classpath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Prefer the PostgreSQL JDBC driver first—it’s often already on your classpath | |
| Prefer the PostgreSQL JDBC driver first, it’s often already on your classpath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also:
| Prefer the PostgreSQL JDBC driver first—it’s often already on your classpath | |
| Prefer the PostgreSQL JDBC driver first—it’s often already in your classpath |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved with 59b8ea3, thanks.
docs/connect/java/postgresql-jdbc.md
Outdated
| Invoke-WebRequest https://repo1.maven.org/maven2/org/postgresql/postgresql/42.7.8/postgresql-42.7.8.jar -OutFile postgresql-42.7.8.jar | ||
| ``` | ||
| ::: | ||
| Invoke program. Needs Java >= 25 ([JEP 330]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PostgreSQL-JDBC works with Java >= 1.8, and the given command does not need JEP 330 afaik.
| Invoke program. Needs Java >= 25 ([JEP 330]). | |
| Invoke program. Needs Java >= 1.8. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is important information for some. We will highlight runtime version compatibility more prominently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed inside 0fff874, thank you.
matriv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx, Don't have more comments myself.
Previously, the docs stated Java 25 would be required, which isn't true.
docs/connect/java/cratedb-jdbc.md
Outdated
| Invoke-WebRequest https://repo1.maven.org/maven2/io/crate/crate-jdbc-standalone/2.7.0/crate-jdbc-standalone-2.7.0.jar -OutFile crate-jdbc-standalone-2.7.0.jar | ||
| ``` | ||
| ::: | ||
| Invoke program. Needs Java >= 25 ([JEP 330]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
JEP 330
Hi. We see that JEP 330 landed with Java 11 already, so this comment is certainly misguided; thanks for spotting it.
Apparently, the fourth mode for the Java launcher to launch a class declared in a source file has been added a long time ago already.
java HelloWorld.java
We may have been confused about it, because we don't launch Java programs so often, so we didn't know about this option before, expecting that launching a program requires an incantation of javac beforehand.
JEP 445
However, the example above uses this feature already:
Java 21 introduces unnamed classes to make the
classdeclaration implicit. Alsopublic static void main(String args[])is no longer required, to reduce boilerplate for single-class programs.
So, this effectively needs JEP 445 which landed with Java 21. Comparing those snippets, it saves a few bytes, class noise, a whole indentation level, and any "naming things" obstacles regarding Java specifics in file name vs. class name discrepancies.
public class HelloWorld {
public static void main(String[] args) {
System.out.println("Hello, World!");
}
}void main() {
System.out.println("Hello, World!");
}
docs/connect/java/cratedb-jdbc.md
Outdated
| Invoke-WebRequest https://repo1.maven.org/maven2/io/crate/crate-jdbc-standalone/2.7.0/crate-jdbc-standalone-2.7.0.jar -OutFile crate-jdbc-standalone-2.7.0.jar | ||
| ``` | ||
| ::: | ||
| Invoke program. Needs Java >= 25 ([JEP 330]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts / Evaluation
So, is it worth to sacrifice backward compatibility for the canonical poster example?
My thoughts were we indeed could do it, because the other spot where canonical code examples are presented, in this case cratedb-examples:by-language/java-jdbc, displays a full Java program BasicPostgresCrateDB.java anyway.
In this spirit, we wanted to reduce boilerplate to the max here, yet retaining the idea of presenting an executable example. It's a narrow path which sometimes needs modern language features, or utility programs like uv for elegantly launching Python code snippets, where the user/reader mostly needed to whip up a whole programming environment (Python virtualenv) before even getting started.
Following this idea and the modernization efforts of upstream software authors who work on such ergonomic enhancements, we have been aiming to present "modern" variants of how to minimally invoke arbitrary code snippets on our documentation as well.
Evaluation / Next steps
We can collectively decide if it's okay to present the modern variants while sacrificing backwards compatibility with previous programming language versions, but sure we should present information which accompanies older versions as well.
Let me come up with another iteration on this document to accomodate a better balance for Java < 21, so you can evaluate and decide if you like it.
docs/connect/java/cratedb-jdbc.md
Outdated
| Invoke-WebRequest https://repo1.maven.org/maven2/io/crate/crate-jdbc-standalone/2.7.0/crate-jdbc-standalone-2.7.0.jar -OutFile crate-jdbc-standalone-2.7.0.jar | ||
| ``` | ||
| ::: | ||
| Invoke program. Needs Java >= 25 ([JEP 330]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crate-jdbc requires >= 11
We will certainly add this remark about the effective runtime dependency separately and more prominently, thank you.
docs/connect/java/postgresql-jdbc.md
Outdated
| Invoke-WebRequest https://repo1.maven.org/maven2/org/postgresql/postgresql/42.7.8/postgresql-42.7.8.jar -OutFile postgresql-42.7.8.jar | ||
| ``` | ||
| ::: | ||
| Invoke program. Needs Java >= 25 ([JEP 330]). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this is important information for some. We will highlight runtime version compatibility more prominently.
docs/connect/java/testing.md
Outdated
| (java-testing)= | ||
|
|
||
| # Software testing | ||
|
|
||
| For testing Java applications against CrateDB, see also documentation | ||
| about {ref}`java-junit` and {ref}`testcontainers`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While being at it, let's unflatten this page a bit more, like others are doing it, probably by using information cards.
Secondly, another possibility is to go further and also relocate the content currently slotted into the topics section, which is bound to be dissolved anyway. Here, closer to its primary topic domain Java, the content would be located much better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Secondly, another possibility is to go further and also relocate the content currently slotted into the
topicssection, which is bound to be dissolved anyway.
Excellent. Implemented per ff18c5b.
-- https://cratedb-guide--402.org.readthedocs.build/connect/java/testing.html
docs/connect/java/cratedb-jdbc.md
Outdated
| :::{include} ../_cratedb.md | ||
| ::: | ||
| Download JAR file. | ||
| ```shell | ||
| wget https://repo1.maven.org/maven2/io/crate/crate-jdbc-standalone/2.7.0/crate-jdbc-standalone-2.7.0.jar | ||
| ``` | ||
| :::{dropdown} Instructions for Windows users | ||
| If you don't have the `wget` program installed, for example on Windows, just | ||
| download the JAR file using your web browser of choice. | ||
| If you want to use PowerShell, invoke the `Invoke-WebRequest` command instead | ||
| of `wget`. | ||
| ```powershell | ||
| Invoke-WebRequest https://repo1.maven.org/maven2/io/crate/crate-jdbc-standalone/2.7.0/crate-jdbc-standalone-2.7.0.jar -OutFile crate-jdbc-standalone-2.7.0.jar | ||
| ``` | ||
| ::: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dropdown reduces clutter/noise significantly already, still it obstructs the reading flow. Consider three things for the next iteration:
a) Provide the download link also per HTML link anchor <a href=""></a>, to better support browser users as already mentioned, freeing them from needing to copy/paste the URL.
b) Relocate all special considerations (wget or Invoke-WebRequest) to the bottom of the page.
c) Alternatively to b), add instructions for both operation system variants (wget and Invoke-WebRequest) to the dropdown, but keep it in this section without relocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've relocated all the instruction noise to the bottom of the pages per dcce170 now, so the synopsis is more in the focus, followed by (typical) installation instructions, with the quickstart example only at the bottom of the page. I think the pages are much more approachable now, and convey their information in the right order.
-- https://cratedb-guide--402.org.readthedocs.build/connect/java/postgresql-jdbc.html
-- https://cratedb-guide--402.org.readthedocs.build/connect/java/cratedb-jdbc.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More command noise reduction per 5d5de24, we think information absorption is pretty swift now.
Previously, the docs stated Java 21 would be okay, which isn't true.
About
Preview
https://cratedb-guide--402.org.readthedocs.build/connect/java/
/cc @zolbatar, @kneth, @surister